-
-
Notifications
You must be signed in to change notification settings - Fork 2
4.0 | Wiki/Advanced-Usage: add the cumulative exit codes #51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
4.0 | Wiki/Advanced-Usage: add the cumulative exit codes #51
Conversation
- Added the cumulative exit codes as documented in the interim proposal (PHPCSStandards/PHP_CodeSniffer#184 (comment)). This ensures a complete exit code documentation where users and devs can easily read exactly what the cumulative exit codes are. - Also added another description to the `0` exit code to describe when issues are fixed with no issues remaining, because "clean code base" could just be interpreted as no issues at all.
=== This is an auto-generated comment === Thank you for your PR. Please review the resulting final markdown files via the created artifact. N.B.: the above link will automatically be updated when this PR is updated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yCodeTech Hi Stu, thanks for this PR.
What with the cumulative exit codes table in this PR, I wonder if line 527 "Example ..." can be removed. What do you think ?
Oh and one small nitpick: to have more readable source code for the wiki, I tend to prefer tables to be properly aligned. The new table isn't.
If it helps, the https://table-magic.guntrip.co.uk/ website is a nice way to get the formatted table - paste the original table into the "Markdown" tab, uncheck and check again the "Table formatter" checkbox, copy & paste & done ;-)
@jrfnl Hi Juliette. Yeah I wasn't sure whether to remove the "Example..." line or not. But because I added the 1+2 etc. into the descriptions (which I wasn't initially going to do), I guess the example line could be removed. Though it depends if the tables alone would be enough to describe the cumulative behaviour? I can remove the example on 527 if you think it's now redundant? Oops my bad. While making sure my addition to the |
Could potentially add an extra line to the introduction of the exit codes to describe the cumulative behaviour. Perhaps something like: "...exit codes are cumulative, meaning some situations can be added on top of one another and will increase the exit code. The cumulative codes are composed..." What do you think? |
@yCodeTech Looking at your responses and looking at the page again - how about instead of changing the intro line, we combine the "table header" line (for the new table) with the example line to say something along the lines of what you are hinting at with the text sample above ? Would that work ? |
@jrfnl Ok so if I'm understanding your comment correctly, instead of having the "Example..." on line 527 and the "Cumulative exit codes:" on 529, we'd have a something like what I wrote above. If that's the case, it would work and the new line 527 could read: "The exit codes in some situations can be combined, resulting in a new exit code. These cumulative exit codes are as follows:" We could also change the cumulative table slightly to add a new column for the exit codes that are added together instead of having them in brackets in the description? |
@yCodeTech Sounds good to me! |
- Added new column to the cumulative exit codes table to define the codes that were combined, instead of defining them in brackets in the descriptions. - Removed the now redundant cumulative exit code example. - Changed the cumulative table header to describe the cumulative behaviour.
@jrfnl I've committed the changes. But regarding the new column, I'm not entirely sure what the best column title should be. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yCodeTech I think it's clear as it is now. And we can always iterate on this change again later.
Thanks for contributing!
Added the cumulative exit codes as documented in the interim proposal. This ensures a complete exit code documentation where users and devs can easily read exactly what the cumulative exit codes are.
Added another description to the
0
exit code to describe when issues are fixed with no issues remaining, because "clean code base" could just be interpreted as no issues at all.Description
Related issues/external references
PHPCSStandards/PHP_CodeSniffer#184 (comment)
PR checklist
_Sidebar.md
file.